Skip to content

ci: run build-test-push on all PRs, not just same-repo#39

Merged
david-banon-tecnativa merged 1 commit into
Tecnativa:masterfrom
moduon:ci-build-on-prs
Jul 14, 2026
Merged

ci: run build-test-push on all PRs, not just same-repo#39
david-banon-tecnativa merged 1 commit into
Tecnativa:masterfrom
moduon:ci-build-on-prs

Conversation

@yajo

@yajo yajo commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Fork PRs previously skipped the build-test-push job entirely,
preventing contributors from getting build and test feedback.

Removing the same-repo restriction lets all PRs run the pipeline.
The DockerHub push step was already guarded by credential checks,
and a guard was added to the GHCR push step to prevent failures
on forks where GITHUB_TOKEN is read-only.

Comment thread .github/workflows/ci.yaml Outdated
Comment thread .github/workflows/ci.yaml Outdated
@yajo yajo force-pushed the ci-build-on-prs branch from 8ce6e42 to edc7219 Compare June 22, 2026 10:53
@yajo

yajo commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review, @ljmnoonan! Both points are addressed in the latest revision:

  1. github.actorgithub.repository_owner: You're right that github.actor could be confusing in org fork scenarios. Reverted to github.repository_owner — it works identically with GITHUB_TOKEN and avoids the concern.

  2. Fork PR push with bot credentials: Good catch. The step now exposes BOT_TOKEN and BOT_LOGIN as env vars and uses them in the if: condition: the push runs on fork PRs too when both bot credentials are available, and is only skipped when GITHUB_TOKEN would be read-only (fork PR without bot creds). Same-repo PRs still push via GITHUB_TOKEN as before.

@ljmnoonan

Copy link
Copy Markdown
Contributor

@yajo, this looks perfect! BTW, CI is failing because of upstream changes to alpine. I fixed in #40

@david-banon-tecnativa

Copy link
Copy Markdown
Contributor

Hi, thanks for putting in the work for this PR.
Can you rebase now that the issue making tests fail is resolved?

Fork PRs previously skipped the build-test-push job entirely,
preventing contributors from getting build and test feedback.

The GHCR push step used || between independent secrets:
BOT_TOKEN || GITHUB_TOKEN and BOT_LOGIN || repository_owner.
When only one BOT_* secret was set, credentials became a mismatched
pair, causing a denied: denied authentication failure from ghcr.io.

Changes:
- Remove same-repo restriction from job condition so all PRs
  run the pipeline.
- Use && to ensure both BOT_TOKEN and BOT_LOGIN must exist
  together to be used; otherwise fall back to the always-
  available GITHUB_TOKEN + github.repository_owner pair.
- Expose BOT_TOKEN and BOT_LOGIN as env vars so they can be
  evaluated in the if: condition.
- Allow GHCR push on fork PRs when bot credentials are
  available; skip only when GITHUB_TOKEN would be read-only
  (fork PR without BOT_TOKEN/BOT_LOGIN).
- Use github.repository_owner as fallback username instead of
  github.actor to avoid confusion in org fork scenarios.
@yajo yajo force-pushed the ci-build-on-prs branch from edc7219 to 85f46b5 Compare July 14, 2026 08:55
@yajo

yajo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Rebased.

@david-banon-tecnativa david-banon-tecnativa merged commit e79329a into Tecnativa:master Jul 14, 2026
11 checks passed
@yajo yajo deleted the ci-build-on-prs branch July 14, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants